Repository navigation
feat(mosaic): wire up user profile emails and phone numbers - #9937
alexcarpenter wants to merge 78 commits into
Conversation
🦋 Changeset detectedLatest commit: 1eb657c The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
💤 Files with no reviewable changes (1)
Included review availability: This review used your included allowance. 4 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 8 reviews per hour. 📝 WalkthroughWalkthroughThe account section adds contact access rules and email and phone verification flows using code, link, and SSO methods. It updates form error handling, test API endpoints and fixtures, and profile-picture upload and removal controls. Tests and Swingset stories cover the updated flows, contact ordering, account restrictions, and related UI states. Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~50 minutes Merge Risk: 🟡 Moderate · up to The test deletion does not resolve the earlier concerns. A phone verification send failure may show no message. The shared test harness may have duplicate function definitions that block compilation. The fake API can also return stale or colliding data. Resolve these before merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
alexcarpenter
changed the title
alexcarpenter
changed the title
c14b88b to
0f3c4e2
Compare
c3c548f to
9a7c43d
Compare
Legacy reads the geo-IP country off the Clerk instance and seeds the phone input with it. Mosaic's PhoneInput took a defaultCountry but nothing passed one, so it always started on 'us' and a non-US user typing a national number submitted a +1 number. The account section's model now narrows clerk.__internal_country through a new toCountryIso and threads it down to the input as defaultCountry. The fake FAPI gained a country seed that serves x-country, so the feature test exercises the real clerk-js path that populates the value.
The confirmation block rendered the raw rejection message and never reached the error catalog, so a mapped code like action_blocked lost its copy and a SaveError built from UNEXPECTED_ERROR surfaced the developer string "Save failed". Nine views share the block, so all of them showed it. The machine now holds the LocalizableError and the hook localizes it with errorText, mirroring how useForm does it. Reading a rejection into a LocalizableError is the same shape fourteen other sites hand-roll, so it lands in utils/form-error.ts as toLocalizableError rather than here; the remaining sites still need migrating. A plain Error still shows its own message, which keeps working for the callers that localize before throwing.
The helper clicked the field as soon as it mounted, but the verify step disables it while the code is being sent and the disabled style sets pointer-events: none, so on a slow runner the click was refused.
…or it The SSO step prepared the verification on entry and the Connect button read the redirect URL off the resource when clicked, so a click that beat the response found no URL and silently did nothing. Connect now performs the prepare itself and navigates to the URL it answers with, so there is nothing to race and a failure is reported instead of swallowed.
…ile-email-link-sso
…ile-email-link-sso
A field-scoped prepareVerification failure landed in the form error's fields, which nothing on the verify step renders, so the user saw an empty error. The email path already saves without a field list. Also corrects the swingset add-phone story to the fixture's onCreated/onVerified contract, and the useForm failure contract to name SaveError.
…ile-email-link-sso # Conflicts: # packages/mosaic/src/features/user-profile/__tests__/user-profile-account-section.integration.test.tsx # packages/mosaic/src/features/user-profile/__tests__/user-profile-email-actions.test.tsx # packages/mosaic/src/features/user-profile/__tests__/user-profile-phone-actions.test.tsx
…onent Follows the convention landed in #10076: a feature test lives next to the component it renders.
Ephem
left a comment
Ephem
left a comment
There was a problem hiding this comment.
Just did a quick skim for some obvious stuff.
| } | ||
|
|
||
| /** Reads what any rejection says, keeping a failed save's code so the copy stays localizable. */ | ||
| export function toLocalizableError(cause: unknown): LocalizableError { |
There was a problem hiding this comment.
This should disappear when you rebase (just commenting so it doesn't accidentally get missed).
| const verification = useUserProfileAddEmailController({ | ||
| username, | ||
| onCreate: onCreateEmail, | ||
| }); |
There was a problem hiding this comment.
Controller in a view seems off, plus a bunch of other logic? Same with the phone-row.
Is this some kind of intermediary state that you plan to tackle in a follow up, or how you intended it?
| items, | ||
| onSetPrimary, | ||
| }: UserProfileSetPrimaryControllerOptions): UserProfileSetPrimaryController { | ||
| const form = useForm({ |
There was a problem hiding this comment.
Should this use usePendingAction instead now that it exists? Same in user-profile-picture.controller.ts
…ile-email-link-sso # Conflicts: # packages/mosaic/src/blocks/confirmation/confirmation.controller.test.ts # packages/mosaic/src/blocks/confirmation/confirmation.controller.ts # packages/mosaic/src/features/user-profile/__tests__/user-profile-account-section.model.test.tsx # packages/mosaic/src/features/user-profile/__tests__/user-profile-picture.controller.test.ts # packages/mosaic/src/features/user-profile/user-profile-account-section/user-profile-picture.controller.ts # packages/mosaic/src/utils/errors.ts # packages/swingset/src/stories/user-profile-account-section.stories.tsx
Give the email and phone rows their own controllers built on usePendingAction, and delete user-profile-set-primary.controller.ts, which reimplemented that hook as a single-field form. The picture controller moves onto the same hook.
This branch was successfully deployed
2 participants
Description
Wires the email and phone lists in the Mosaic user profile to Clerk, matching the legacy
EmailsSectionandPhoneSection. On top of #9844. Supersedes #9927 and #9936, which are folded in here.The diff is large but front-loaded with churn: over half of it is tests, and the production change nets roughly +300 lines, almost all inside
features/user-profile/user-profile-account-section/. The reading order below is the short path through it.Behavior
useForm, and a contact left pending can be verified from its row menu. The dialog opens on the step for the instance's verification method: code, email link, or enterprise SSO.useFormis now the one owner of pending state and error copy across the section, which retires the per-contact "Unable to set the primary…" strings. The enterprise accounts Connect button becomes aSubmitButton, so it holds its label while the connection runs.Reading order
user-profile-account-section.types.tsUserProfileEmailVerifier,UserProfilePhoneVerifier, and thecode | link | ssoverification union.user-profile-account-section.model.tsuser-profile-add-email.controller.tsuser-profile-add-email.dialog.tsxemail,verify,link,sso). It absorbs the two standalone verify dialogs this PR deletes.user-profile-add-phone.controller.tsuser-profile-set-primary.controller.tsuser-profile-email-row.view.tsx,user-profile-phone-row.view.tsxuser-profile-account-section.utils.tstoContactAccessgating that decides what a row may offer.user-profile-account-section.feature.test.tsxChurn worth skimming rather than reading
user-profile-verify-email-link.{dialog,messages,styles}anduser-profile-verify-email-sso.{dialog,messages,styles}, plus their two swingset fixtures and their two localization namespaces.user-profile-account-section.model.test.tsx, three*.integration.test.tsx, and the two verify-dialog tests all come out.user-profile-picture.controller.tsdrops a hand-rolled pending/error/in-flight ref foruseForm, which is why it shrinks.Changes outside the section, and why each is here
components/form/form.machine.ts— a banner now clears when a submit succeeds rather than when one starts, so a retry does not flash an empty error.utils/form-error.ts— addstoLocalizableError, so a rejection keeps its Clerk error code instead of flattening to a string.blocks/confirmation/confirmation.controller.ts— the remove-contact confirmation localizes the reason the server refused, instead of printing the raw message.components/phone-input/— exportstoCountryIso, so the model can turnclerk.__internal_countryinto the input's default country.primitives/menu/menu.test.tsx— regression test: the row menu has to hold its items at their last frame while it exits, or "Set as primary" disappears mid-animation after it is clicked.hooks/use-list-removal-focus.ts— exposestrigger(id), so a verify dialog can return focus to the row that opened it.__tests__/feature/fapi.ts,__tests__/feature/fake-fapi.ts— the shared feature-test harness grows attribute overrides,fapiPhoneNumber, and the phone and email verification endpoints.features/user-button/__tests__/user-button.feature.test.tsx— uses that newfapiPhoneNumberhelper in place of an inline literal.Known gaps
userProfileUrl#/verify. Mosaic has no routing yet, so the base is always the instance's profile URL with a hash path, where legacy derives both from the routing mode, and nothing in Mosaic serves/verifyyet. A TODO in the model points at feat(mosaic): add MosaicRoutingProvider and useMosaicRoutes #9843.EmailAddressResourcecarries onlymatchesSsoConnection. clerk_go#22625 adds theenterprise_connectionsit needs.None of this is exported yet, so the changeset is empty.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change